Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ImmutableArray readonly #44640

Merged
merged 6 commits into from
Nov 17, 2020

Conversation

huoyaoyuan
Copy link
Member

Closes #334 .

I noticed that #44629 does not come with api review. I think this can be done similarly.
As an implementation requirement, this PR brings a new reference of S.R.CS.Unsafe to netstandard1.0 target. It this acceptable?

@ghost
Copy link

ghost commented Nov 13, 2020

Tagging subscribers to this area: @eiriktsarpalis, @jeffhandley
See info in area-owners.md if you want to be subscribed.

Issue Details
Description:

Closes #334 .

I noticed that #44629 does not come with api review. I think this can be done similarly.
As an implementation requirement, this PR brings a new reference of S.R.CS.Unsafe to netstandard1.0 target. It this acceptable?

Author: huoyaoyuan
Assignees: -
Labels:

area-System.Collections

Milestone: -

@@ -121,7 +122,7 @@ public static bool Update<T>(ref ImmutableArray<T> location, Func<ImmutableArray
Requires.NotNull(transformer, nameof(transformer));

bool successful;
T[]? oldArray = Volatile.Read(ref location.array);
T[]? oldArray = Volatile.Read(ref Unsafe.As<ImmutableArray<T>, T[]?>(ref location));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this use just Unsafe.AsRef to just cast-away the readonly-ness? Unsafe.As is too big hammer.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense

@jkotas
Copy link
Member

jkotas commented Nov 13, 2020

PR brings a new reference of S.R.CS.Unsafe

This may be a problem for Roslyn. @jaredpar ?

@jaredpar
Copy link
Member

This may be a problem for Roslyn

It's a headache to update but not a blocker.

@eiriktsarpalis
Copy link
Member

LGTM. Thank you for your contribution.

@eiriktsarpalis eiriktsarpalis merged commit 223fd3a into dotnet:master Nov 17, 2020
@huoyaoyuan huoyaoyuan deleted the readonly-immutable-array branch November 17, 2020 13:20
@ghost ghost locked as resolved and limited conversation to collaborators Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API proposal: Make ImmutableArray readonly struct
5 participants